-
Notifications
You must be signed in to change notification settings - Fork 4
108 exclude list #110
base: main
Are you sure you want to change the base?
108 exclude list #110
Conversation
9892660 to
fd538c8
Compare
409cb1a to
fd69f2d
Compare
| const config = createWebpackConfig(urc); | ||
| expect(config.module.rules[0].oneOf[1].include).toBeUndefined(); | ||
| }); | ||
| }); |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Could use a couple more tests:
- Error case if user provides both include and exclude lists
- Case where use provides their own set of exclude modules
|
Okay, this is now working. @kepta could you give this a look and lmk if the concept seems okay? If so I'll wrap up tests and docs. |
|
It would be better to update readme to tell users that |
|
One idea that could prevent the need for mutually exclusive configuration properties: make a breaking change to the For example, with an object you could do something like this: compileNodeModules: { include: ['*'], exclude: ['mapbox-gl'] }
compileNodeModules: { exclude: ['mapbox-gl'] } // many * is the default include: valueOne other opinion if you're still brainstorming: I'm wondering if the complexity of trying to distinguish the GL JS version is more trouble than it's worth, since the underreact user should be aware of their GL JS version and can decide to compile or not compile based on that. |
dereklieu
left a comment
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I agree with the feedback here to either consolidate or document the include/exclude interface.
I also agree with removing special consideration for gl-js v2 in the configuration generator. This could be something to call out in the read me though, since it's true that many consumers of this project may also use gl-js.
kepta
left a comment
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
One option is to directly export the include webpack property and let folks deal with it directly. We can add an example in the README on the best way to handle with gl-js versions.
That said, I am okay with any of the above approaches but would prefer whichever adds the least complexity to this aging codebase.
|
I suggest just mapping |
This is a more abstracted fix for #108, an alternative to #109. Here, user may provide an
excludeCompileNodeModulesoption as an alternative tocompileNodeModules, in order to exclude specific modules. By default, GL JS 2+ gets placed in the excludeList. Needs tests and docs.